-
Notifications
You must be signed in to change notification settings - Fork 696
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow "cycles" through test suite dependencies (VERSION TWO) #3290
Conversation
Of course, this test currently fails.
When a test suite has a dependency which is not shared with the main library, we can consider it independent. This addresses haskell#1575 to some degree. Suppose * optparse-applicative has a test suite that depends on tasty * tasty has a (regular) dependency on optparse-applicative We can resolve this by linking optparse-applicative's test suite against an older version of itself. This only works provided that optparse-applicative's test suite does not declare optparse-applicative as a dependency (and instead just compiles in the modules from the src/ directory or whatever). If the test suite did declare the library as a dependency, then clearly the test suite needs to be built against _this_ version of the library; it would be terribly confusing if the test suite got built against an older version. But if the test suite gets built against the library itself, then if the test suite also needs tasty, we cannot pick two different versions in the same application (yet). In this commit we add the appropriate qualifiers; however, the resulting install plan will now be rejected by the internal validity check. That's next up.
For test suites etc. we might want to allow for inconsistent dependencies. For instance, in the optparse-applicative -> tasty -> optparse-applicative dependency cycle we might want to allow two versions of optparse-applicative in the same executable (one that is the library-under-test and one used internally by tasty).
Details of the test in the README.
Signed-off-by: Edward Z. Yang <[email protected]>
If the tests pass, can you please squash fix-up commits with the original ones? Would be nice to avoid having broken commits in history. |
There's still a bug in this PR though,
UPDATE: and then the third time I run it, the dependency solver fails altogether:
@23Skidoo Of course. |
The log from the third run looks like #3203. EDIT: I think there are multiple
|
(I guess, at some point, we should move this to a branch on |
Signed-off-by: Edward Z. Yang <[email protected]>
90cf951
to
85c7448
Compare
So I think I won't delay the first 1.24 release for this. If @kosmikus thinks that this PR should go into 1.24, I think it can be made a part of a point release. Which means that 1.24 is now good to go. |
Independently of the first 1.24 release, I'd like to understand better what is going on here. There's quite a bit of history, and perhaps someone who has been following that more closely than I have can provide a good summary of what the status here is? @ezyang ? |
I don't really know how the patch internally works, but I know that the obvious test for the feature (cabal installing a package with a cyclic dependency in a sandbox) doesn't work properly. So someone needs to dig in and figure out why it's failing and then fix it. |
@edsko, can you please look into it? |
@23Skidoo the solver part of this is working, as far as I can tell. I'm not sure what's going on with the sandbox, some additional check somewhere I think. I don't know when I'll get a chance to look at that; perhaps somebody who is more familiar with the sandboxing stuff would be able to identify the problem more quickly? |
I'll try to take a look at this next month. |
@ezyang I wanted to to try this PR to see whether it fixes the issues I have with a couple of packages ( |
Closing in favour of #3402. |
@23Skidoo thanks! |
This is a version of #3232 updated for HEAD.
CC @edsko